Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(releases): use get_or_create instead of sub transaction #77453

Merged
merged 2 commits into from
Sep 17, 2024

Conversation

JoshFerge
Copy link
Member

reduce the amount of sub transactions by using get_or_create instead of catching integrity errors. also removes the ff flag for now. will keep the ff flag defined incase i make any more radical changes

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Sep 12, 2024
@JoshFerge JoshFerge requested a review from a team September 12, 2024 22:51
Comment on lines 60 to 61
create_repositories(commit_list, release)
create_commit_authors(commit_list, release)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure we don't still want these inside of the lock, but not the transaction?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added inside of lock, but not transaction.

@JoshFerge JoshFerge enabled auto-merge (squash) September 17, 2024 15:04
@JoshFerge JoshFerge merged commit 56a2c91 into master Sep 17, 2024
50 checks passed
@JoshFerge JoshFerge deleted the jferg/more-opts branch September 17, 2024 15:37
Copy link

sentry-io bot commented Sep 17, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ IntegrityError: UniqueViolation('duplicate key value violates unique constraint "sentry_releasecommit_release_id_... sentry.tasks.commits.fetch_commits View Issue

Did you find this useful? React with a 👍 or 👎

@JoshFerge JoshFerge added the Trigger: Revert Add to a merged PR to revert it (skips CI) label Sep 17, 2024
@getsentry-bot
Copy link
Contributor

PR reverted: 944ae32

getsentry-bot added a commit that referenced this pull request Sep 17, 2024
JoshFerge added a commit that referenced this pull request Sep 17, 2024
we observed SENTRY-3F2A during the rollout of
#77453.

This didn't make sense until i realized our redis lock is only 10
seconds right now, and this function can take longer than that. We
increase the max timeout to 10 minutes, which should be plenty for
99.99+% of `set_commits` calls.
harshithadurai pushed a commit that referenced this pull request Sep 19, 2024
we observed SENTRY-3F2A during the rollout of
#77453.

This didn't make sense until i realized our redis lock is only 10
seconds right now, and this function can take longer than that. We
increase the max timeout to 10 minutes, which should be plenty for
99.99+% of `set_commits` calls.
harshithadurai pushed a commit that referenced this pull request Sep 19, 2024
we observed SENTRY-3F2A during the rollout of
#77453.

This didn't make sense until i realized our redis lock is only 10
seconds right now, and this function can take longer than that. We
increase the max timeout to 10 minutes, which should be plenty for
99.99+% of `set_commits` calls.
@github-actions github-actions bot locked and limited conversation to collaborators Oct 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components Trigger: Revert Add to a merged PR to revert it (skips CI)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants